Skip to content

fix(bridge): send room approval request_id - #121

Merged
TheSmokeDev merged 1 commit into
TheSmokeDev:mainfrom
danclaw93:fix/room-approval-request-id
Sep 7, 2026
Merged

fix(bridge): send room approval request_id#121
TheSmokeDev merged 1 commit into
TheSmokeDev:mainfrom
danclaw93:fix/room-approval-request-id

Conversation

@danclaw93

Copy link
Copy Markdown
Contributor

What and why

Room-scoped approvals require the exact request_id, but respond_to_approval() sends that identifier as approvalId. Current Hermes therefore rejects spoken approval responses with HTTP 400 approval_request_required, leaving the pending action blocked.

Send the approval event identifier under the Hermes run API's request_id field and update the surrounding contract documentation and regression expectation.

Fixes #120

How to test

The regression test was changed first and failed on main because the emitted payload still contained approvalId:

AssertionError: {'choice': 'once', 'approvalId': 'req-77'} != {'choice': 'once', 'request_id': 'req-77'}

With this fix:

env -u TALK_PREFER_CODEX_OAUTH uv run --extra dev pytest -q tests/test_approval_bridge.py::test_respond_to_approval_carries_the_request_id_when_given
env -u TALK_PREFER_CODEX_OAUTH uv run --extra dev pytest -q
uv run --extra dev ruff check .
1 passed
1671 passed, 49 skipped, 5 xfailed
All checks passed!

An integration check using the real Talk request builder and current Hermes approval handler also returned HTTP 200 with resolved: 1 for the room-scoped request.

Platforms

  • Windows
  • Linux
  • macOS

Live receipt

Not required: this changes the local Hermes run-approval API payload, not a provider lane, credential-resolution path, or delegation implementation. The Talk-to-Hermes handler integration was exercised locally with an inert pending approval and temporary HERMES_HOME.

Checklist

  • One logical change; tests ride with it, not behind it
  • pytest -q and ruff check . pass locally on the pinned ruff
  • Commits follow Conventional Commits with a hermes-talk scope
  • No auth-store writes outside the documented Codex refresh; tokens reach only the provider's own host
  • Nothing secret in logs, receipts, spoken sentences, or test fixtures
  • Every spoken sentence I added or changed claims only what an artifact proves
  • Docs updated where behaviour changed — code contract comments updated; user-facing operating and voice docs are N/A
  • CHANGELOG.md [Unreleased] entry added

@danclaw93
danclaw93 force-pushed the fix/room-approval-request-id branch from 3df3e42 to b18b60f Compare September 4, 2026 06:00
@TheSmokeDev
TheSmokeDev merged commit 097f264 into TheSmokeDev:main Sep 7, 2026
11 checks passed
@TheSmokeDev

Copy link
Copy Markdown
Owner

thanks @danclaw93, merged. you were right: the host reads request_id, and approvalId was never a field it knew, so the room lane had been broken since 0.15.0. shipped in 0.17.1 with your name in the changelog.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Room-scoped approvals fail because hermes-talk sends approvalId instead of request_id

2 participants